Add command to add JDK to settings #4162
Conversation
rgrunber
left a comment
rgrunber
left a comment
There was a problem hiding this comment.
Seems mostly fine. Just be aware this overlaps with https://github.com/microsoft/vscode-java-pack/blob/main/src/java-runtime/index.ts and it probably would have been ideal to move that page here.
src/javaRuntimes.ts
Outdated
|
|
||
| export namespace JavaRuntimes { | ||
| export async function initialize(context: vscode.ExtensionContext): Promise<void> { | ||
| context.subscriptions.push(vscode.commands.registerCommand('java.runtimes.add', async () => { |
There was a problem hiding this comment.
I'd probably add "java.runtimes.add" to Commands.
fbricon
left a comment
fbricon
left a comment
There was a problem hiding this comment.
Can you please rebase against the main branch? JavaSE-25 is available now.
src/javaRuntimes.ts
Outdated
| const config = vscode.workspace.getConfiguration('java.configuration').get('runtimes'); | ||
| if (Array.isArray(config)) { | ||
| if (config.some(r => r.path === directory[0].fsPath)) { | ||
| vscode.window.showErrorMessage(`JDK Directory ${directory[0].fsPath} already configured`); |
There was a problem hiding this comment.
No you need to ensure uniqueness of the jre name.
It's OK having the same JDK used for 2 runtimes, eg. use JDK 11 for JavaSE-11 and JavaSE-1.8
src/javaRuntimes.ts
Outdated
| if (config.some(r => r.path === directory[0].fsPath)) { | ||
| vscode.window.showErrorMessage(`JDK Directory ${directory[0].fsPath} already configured`); | ||
| } else { | ||
| const name = await vscode.window.showQuickPick(getSupportedJreNames(), { |
There was a problem hiding this comment.
I would improve it by:
- detecting the java version of the selected path
- preselect the matching execution environment name
- filter out incompatible environments, i.e don't show JavaSE-22 and above if you selected a JDK 21.
There was a problem hiding this comment.
also pre Java 8 support has been dropped over a year ago. Can you remove the J2SE-1.5 ... JavaSE-7 values in package.json
src/javaRuntimes.ts
Outdated
| vscode.workspace.getConfiguration('java.configuration').update('runtimes', config, vscode.ConfigurationTarget.Global); | ||
| vscode.window.showInformationMessage(`JDK Directory ${directory[0].fsPath} added`); |
There was a problem hiding this comment.
should be executed only if the quickpick was complete. If I press ESC while the wizard is open, I see the "...added" message.
Signed-off-by: Thomas Mäder <[email protected]>
Signed-off-by: Thomas Mäder <[email protected]>
Signed-off-by: Thomas Mäder <[email protected]>
Signed-off-by: Thomas Mäder <[email protected]>
tsmaeder
force-pushed
the
4161_add_jdk_command
branch
from
3c9b691 to
cbe1b37
Compare
| if (runtime) { | ||
| const config = vscode.workspace.getConfiguration('java.configuration').get('runtimes'); | ||
| if (Array.isArray(config)) { | ||
| const candidates = getSupportedJreNames().filter(name => !config.some(r => r.name === name) && compatible(runtime, name)); |
There was a problem hiding this comment.
- I would not prevent overriding existing JRE Mappings, e.g. I want to switch from one JDK 11.0.0 to 11.0.1
- versions should be sorted by most recent version 1st
3 participants
This adds a VS Code command that
Fixes #4161